Skip to content

Resolve Inheritance Sort Issue#238

Open
ax3l wants to merge 13 commits intopybind:mainfrom
ax3l:test-inheritance-sort-issue
Open

Resolve Inheritance Sort Issue#238
ax3l wants to merge 13 commits intopybind:mainfrom
ax3l:test-inheritance-sort-issue

Conversation

@ax3l
Copy link
Copy Markdown
Contributor

@ax3l ax3l commented Jan 9, 2025

This modifies the inheritance unit test to demonstrate the sorting issue in #231.


Classes in generated .pyi stubs were sorted alphabetically, causing derived classes to appear before their base classes and breaking type checkers. Likewise, class-body dependencies were not sorted properly, creating invalid Python stubs.

Three changes fix this:

  • Parser: use module.__dict__.items() instead of inspect.getmembers() to preserve the pybind11 registration order (definition order)
  • Printer: replace alphabetical sort with a configurable _order_classes() dispatch supporting "definition" (default), "topological" (Kahn's algorithm ensuring bases precede dependent (derived classes, class-body type references)), and "alphabetical"
  • CLI: add --sort-by option to select the class ordering strategy

The topological sort ignores external bases (from other modules) and breaks ties by input position for deterministic output. Cyclic cross-references between classes (e.g. aliases, method signatures) are no inheritance cycles and are already handled by from __future__ import annotations in the generated stubs.

Fixes #231
Closes #294
Closes #275

Based on the approaches in PR #275 by @juelg and PR #294 by @daltairwalter, informed by review feedback from @skarndev and @sizmailov. Combined via Claude Code and reviewed via Codex, and manually.

@ax3l
Copy link
Copy Markdown
Contributor Author

ax3l commented Jan 22, 2026

Continued in #275

@ax3l ax3l closed this Jan 22, 2026
@ax3l ax3l added the bug Something isn't working label Jan 22, 2026
@ax3l ax3l reopened this Apr 2, 2026
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from e083ebc to f1ae578 Compare April 2, 2026 23:46
@ax3l ax3l changed the title Test: Inheritance Sort Issue Resolve Inheritance Sort Issue Apr 3, 2026
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from d93d341 to 5a184c7 Compare April 3, 2026 04:48
This modifies the inheritance unit test to demonstrate
the sorting issue.
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch 2 times, most recently from 1e96b24 to 07662e4 Compare April 3, 2026 05:24
ax3l and others added 2 commits April 2, 2026 22:37
Classes in generated .pyi stubs were sorted alphabetically, causing
derived classes to appear before their base classes and breaking type
checkers.

Three changes fix this:

- Parser: use module.__dict__.items() instead of inspect.getmembers()
  to preserve the pybind11 registration order (definition order)
- Printer: replace alphabetical sort with a configurable _order_classes()
  dispatch supporting "definition" (default), "topological" (Kahn's
  algorithm ensuring bases precede derived classes), and "alphabetical"
- CLI: add --sort-by option to select the class ordering strategy

The topological sort ignores external bases (from other modules) and
breaks ties by input position for deterministic output. Cyclic cross-
references between classes (e.g. aliases, method signatures) are not
inheritance cycles and are already handled by `from __future__ import
annotations` in the generated stubs.

Closes pybind#231

Based on the approaches in PR pybind#275 by @juelg and PR pybind#294 by
@daltairwalter, informed by review feedback from @skarndev and
@sizmailov.

Co-Authored-By: juelg <[email protected]>
Co-Authored-By: daltairwalter <[email protected]>
Co-Authored-By: skarndev <[email protected]>
Co-Authored-By: sizmailov <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from 07662e4 to 13dbbf3 Compare April 3, 2026 05:37
@ax3l ax3l requested review from skarndev and virtuald April 3, 2026 06:09
New `--sort-by` CLI option
ax3l added 2 commits April 7, 2026 21:32
Extend topological class ordering in the printer to account for
executable class-body dependencies, not just inheritance. In addition
to base-class edges, the sorter now treats class aliases and
print-safe field values that reference sibling classes as runtime
dependencies, deduplicates edges, and updates cycle diagnostics to
reflect the broader graph. This keeps generated .pyi files valid
Python without moving declarations out of class bodies, preserving
the conventional stub shape that type checkers expect.

Before:
```py
class ParIterBase:
  level: int

class ParticleContainer:
  name: str
  Iterator = ParIter
  def process(self, arg0: ParIter) -> None: ...

class ParIter(ParIterBase):
  def __init__(self, particle_container: ParticleContainer, level: int) -> None: ...
```

After:
```py
class ParIterBase:
  level: int

class ParIter(ParIterBase):
  def __init__(self, particle_container: ParticleContainer, level: int) -> None: ...

class ParticleContainer:
  name: str
  Iterator = ParIter
  def process(self, arg0: ParIter) -> None: ...
```
@ax3l
Copy link
Copy Markdown
Contributor Author

ax3l commented Apr 8, 2026

I do not think we can easily support robust --sort-by definition if we have class-body dependencies, in addition to base-class edges, not just inheritance. I would remove the option.

Remove the CLI option, we cannot robustly support it with class-body
dependencies.
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from d363ee4 to 51dfa72 Compare April 8, 2026 04:49
ax3l added 2 commits April 7, 2026 21:52
In `pybind11_stubgen/parser/mixins/parse.py`, module traversal now
preserves `module.__dict__` definition order for normal pybind11
exports but also appends lazily exposed members from
`dir()`/`getattr()`, so the PR keeps the ordering fix without
regressing PEP 562-style module attributes.

In `pybind11_stubgen/printer.py`, class dependency edges now use the
first identifier component of a reference, not the last. That fixes
the false-local-match issue for external names like `pkg.other.Foo`,
and it also makes dotted local references like `Outer.Inner` depend
on `Outer`, which is the actual runtime lookup requirement.
@ax3l ax3l requested review from sizmailov and skarndev April 8, 2026 05:15
`handle_class()` now uses a new _iter_class_members() helper that:

- preserves class_.__dict__ definition order for class-local members
- then appends additional visible members from dir()/getattr() so
  inherited or lazily exposed names are still seen

That makes nested class discovery consistent with the earlier
module-level fix, so the printer's topological sort now gets real
definition order as its stable tie-break for nested classes too.
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from 185a5b8 to acca73d Compare April 8, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sorting is causing parent-child relationships to be defined out of order

2 participants